Add support for macOS and iOS - #161
Conversation
ferdnyc
left a comment
There was a problem hiding this comment.
As I mentioned in a couple of review comments, I have no role in this project. You should feel free to ignore any- or everything here, if you prefer, as none of my drive-by feedback carries any official weight whatsoever.
Just getting that out there right from the start.
| @@ -0,0 +1,23 @@ | |||
| { | |||
There was a problem hiding this comment.
Why are there two identical copies of this entire directory in the PR? (/ios/AppIcon.appiconset/ and /ios/Assets.xcassets/AppIcon.appiconset/)
There was a problem hiding this comment.
This was a bug in the icon generation script. I redirected the copy to the build-ios directory.
| "${rc_file}" PROPERTIES | ||
| OBJECT_DEPENDS ${win32_resources}) | ||
| if (MINGW) | ||
| # CMake doesn't know how to process .rc files with MinGW. |
There was a problem hiding this comment.
Why did you delete a bunch of comments that have nothing to do with macOS/iOS?
There was a problem hiding this comment.
Yeah these were accidental removals caused by some command line shenanigans. Forgot about that. I've fixed all the ones you pointed out. Thanks.
| BUNDLE DESTINATION "${CMAKE_INSTALL_APPDIR}") | ||
|
|
||
| if (Qt6_FOUND AND WIN32) | ||
| #manage plugins with windeployqt. |
There was a problem hiding this comment.
Again, deleted comment, unrelated to mac building.
| if (cmd.isEmpty()) { | ||
| QApplication::beep(); | ||
| } else { | ||
| #if !defined(Q_OS_IOS) |
| #if defined(Q_OS_IOS) | ||
| nullptr, QFileDialog::DontUseNativeDialog | ||
| #else | ||
| nullptr | ||
| #endif |
| #with Qt6, winqtdeploy is used to deploy plugins; the method below does not work anymore. | ||
| if (NOT Qt6_FOUND) | ||
| # Install Qt plugins. |
| install_qt_plugin("imageformats" Qt::QSvgPlugin) | ||
| endif() | ||
|
|
||
| # Install MinGW runtime components. |
| copy_to_build_dir(${ADDITIONAL_RUNTIME_LIBS_RELEASE} CONFIGURATIONS Release MinSizeRel RelWithDebInfo) | ||
| copy_to_build_dir(${ADDITIONAL_RUNTIME_LIBS_DEBUG} CONFIGURATIONS Debug) | ||
|
|
||
| # Generate the target that will actually do the copying. |
| - Fakesigned IPAs installed with a free Apple ID via AltStore or Sideloadly expire after 7 days and need to be re-sideloaded. There is no such limitation when installed on a jailbroken device. | ||
| - The iOS file browser is rooted at the app's Documents folder. To make scans accessible, copy them to the ScanTailor Advanced folder via the Files app or Finder file sharing. | ||
| - Directory selection works fully with Magic Keyboard trackpad and keyboard navigation. | ||
| - To override the Qt path: `QT_IOS_DIR=~/Qt/6.x.x/ios QT_MACOS_DIR=~/Qt/6.x.x/macos ./build-ios.sh` No newline at end of file |
There was a problem hiding this comment.
...Why is this last line waaay down here, instead of up where the build command is documented?
| if(IOS) | ||
| # iOS: link only Qt modules; TIFF/PNG/JPEG/ZLIB are bundled inside Qt | ||
| if(Qt6_FOUND) | ||
| target_link_libraries(core | ||
| PUBLIC Qt::Core Qt::Gui Qt::Widgets Qt::Xml Qt::Network Qt::OpenGL Qt::Svg Qt::OpenGLWidgets | ||
| imageproc zones | ||
| fix_orientation page_split deskew select_content page_layout output) | ||
| endif() |
All great feedback. Thank you. ❤️🙏 |
| add_executable( | ||
| ${target_name} WIN32 | ||
| ${gui_only_sources} ${gui_only_ui_files} | ||
| ${resource_files} ${win32_resource_file}) |
There was a problem hiding this comment.
(Apologies, I thought this was included with my first review but it seems I didn't submit it.)
Fun Fact (actually it's kind of an obscure CMake detail): The original add_executable() — including the WIN32 and ${win32_resource_file} — worked cross-platform because CMake is smart enough to ignore things that have nothing to do with the platform being built. It's also smart enough to ignore replaced variables that contain the empty string.
So you can do away with all of this system detection, and just expand the original add_executable() from:
add_executable(
 ${target_name} WIN32 ${gui_only_sources} ${gui_only_ui_files}
${resource_files} ${win32_resource_file})to:
add_executable(
 ${target_name} WIN32 MACOSX_BUNDLE
${gui_only_sources} ${gui_only_ui_files}
${resource_files} ${win32_resource_file})And then change the definition of ${resource_files} so it's empty when building on iOS, instead, just like ${win32_resource_file} will be.
(You might want to flip it the other way around, and change ${resource_files} in the add_executable() to ${executable_resource_files}, instead. Then you can have an if(NOT IOS) that copies ${resource_files} into that variable. i.e.
set(resource_files
resources.qrc
dark_scheme/dark_scheme.qrc
light_scheme/light_scheme.qrc)
list_items_prepend(resource_files "${SCANTAILOR_RESOURCES_DIR}/")
set(executable_resource_files "")
if(NOT IOS)
set(executable_resource_files "${resource_files}")
endif()
# ...
add_executable(
 ${target_name} WIN32 MACOSX_BUNDLE
${gui_only_sources} ${gui_only_ui_files}
${executable_resource_files} ${win32_resource_file})(That way, the original ${resource_files} is still available for use in other parts of the code. If it's needed. Maybe it isn't; I was thinking for install() use or the like, but you don't install compiled-in resources.)
Made these patches since I couldn't find an up to date version for mac and since I use my iPad Pro a lot for working with PDFs and stuff I figured I might as well port it over to iOS/iPadOS as well.
Please let me know if there are any glaring issues. :)